Skip to content

Conversation

b-naber
Copy link
Contributor

@b-naber b-naber commented Sep 20, 2022

Previously the distinction between unevaluated constants in the type-system and in mir was not explicit and a little confusing. Probably better to introduce its own type for that.

r? @lcnr

@rustbot rustbot added A-query-system Area: The rustc query system (https://rustc-dev-guide.rust-lang.org/query.html) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. labels Sep 20, 2022
@rustbot
Copy link
Collaborator

rustbot commented Sep 20, 2022

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

Some changes occurred to MIR optimizations

cc @rust-lang/wg-mir-opt

Some changes occurred in compiler/rustc_codegen_cranelift

cc @bjorn3

Some changes occurred to the CTFE / Miri engine

cc @rust-lang/miri

Some changes occurred in const_evaluatable.rs

cc @lcnr

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Sep 20, 2022
Copy link
Contributor

@lcnr lcnr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice, small nits, then r=me

impl<'tcx> Unevaluated<'tcx> {
// FIXME: probably should get rid of this method. It's also wrong to
// shrink and then later expand a promoted.
#[inline]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, once we've removed visit_mir_constant we should be able to remove this, probably makes sense to do that in yet another pr 😁

@bors
Copy link
Collaborator

bors commented Sep 20, 2022

☔ The latest upstream changes (presumably #102061) made this pull request unmergeable. Please resolve the merge conflicts.

@lcnr lcnr added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Sep 21, 2022
@b-naber b-naber force-pushed the unevaluated branch 2 times, most recently from 23d044f to fad44e0 Compare September 22, 2022 10:39
Copy link
Contributor

@lcnr lcnr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

final nit, then r=me

@lcnr
Copy link
Contributor

lcnr commented Sep 22, 2022

@bors delegate+

@bors
Copy link
Collaborator

bors commented Sep 22, 2022

✌️ @b-naber can now approve this pull request

@@ -605,7 +605,7 @@ impl<'tcx> Inliner<'tcx> {
caller_body.required_consts.extend(
callee_body.required_consts.iter().copied().filter(|&ct| match ct.literal {
ConstantKind::Ty(_) => {
bug!("should never encounter ty::Unevaluated in `required_consts`")
bug!("should never encounter ty::UnevaluatedConst in `required_consts`")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this should be "unexpected ty::Const"

@lcnr
Copy link
Contributor

lcnr commented Sep 23, 2022

@bors r+ rollup=never

maybe perf relevant, probably not 🤷 the queue is pretty short rn

@bors
Copy link
Collaborator

bors commented Sep 23, 2022

📌 Commit a705e65 has been approved by lcnr

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Sep 23, 2022
@bors
Copy link
Collaborator

bors commented Sep 23, 2022

⌛ Testing commit a705e65 with merge 9a963e3...

@bors
Copy link
Collaborator

bors commented Sep 23, 2022

☀️ Test successful - checks-actions
Approved by: lcnr
Pushing 9a963e3 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Sep 23, 2022
@bors bors merged commit 9a963e3 into rust-lang:master Sep 23, 2022
@rustbot rustbot added this to the 1.66.0 milestone Sep 23, 2022
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (9a963e3): comparison URL.

Overall result: no relevant changes - no action needed

@rustbot label: -perf-regression

Instruction count

This benchmark run did not return any relevant results for this metric.

Max RSS (memory usage)

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean1 range count2
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
2.4% [2.0%, 2.9%] 2
Improvements ✅
(primary)
-3.5% [-3.5%, -3.5%] 1
Improvements ✅
(secondary)
-1.7% [-1.7%, -1.7%] 1
All ❌✅ (primary) -3.5% [-3.5%, -3.5%] 1

Cycles

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean1 range count2
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
4.0% [4.0%, 4.0%] 1
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-3.2% [-4.5%, -2.6%] 4
All ❌✅ (primary) - - 0

Footnotes

  1. the arithmetic mean of the percent change 2

  2. number of relevant changes 2

@lcnr lcnr added the A-const-generics Area: const generics (parameters and arguments) label Sep 23, 2022
flip1995 pushed a commit to flip1995/rust that referenced this pull request Oct 6, 2022
Introduce mir::Unevaluated

Previously the distinction between unevaluated constants in the type-system and in mir was not explicit and a little confusing. Probably better to introduce its own type for that.

r? `@lcnr`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-const-generics Area: const generics (parameters and arguments) A-query-system Area: The rustc query system (https://rustc-dev-guide.rust-lang.org/query.html) merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants